Skip to content

feat: GroupKeyHolder for GMS key management#449

Open
moudyellaz wants to merge 13 commits intomainfrom
moudy/feat-group-key-holder
Open

feat: GroupKeyHolder for GMS key management#449
moudyellaz wants to merge 13 commits intomainfrom
moudy/feat-group-key-holder

Conversation

@moudyellaz
Copy link
Copy Markdown
Collaborator

@moudyellaz moudyellaz commented Apr 15, 2026

🎯 Purpose

This PR adds GroupKeyHolder to the wallet for group-owned private PDAs. A group of controllers shares a Group Master Secret (GMS). From it, each controller independently derives identical keys for any PDA the group owns, enabling private multisig and k-of-n patterns over existing PDA-based programs without changing those programs.

⚙️ Approach

key_protocol: GroupKeyHolder (core cryptography)

  • Per-PDA key derivation via SHA256(domain_prefix || gms || pda_seed) producing NSK/NPK/VSK/VPK through the existing derivation chain.
  • epoch counter + ratchet(rotation_salt) forward-hashes the GMS so removed members cannot derive future keys.
  • seal_for(recipient_vpk) / unseal(sealed, own_vsk) encrypts GMS+epoch via ephemeral ECDH + domain-separated KDF + AES-256-GCM for distribution over untrusted channels.
  • Debug impl redacts GMS, raw accessor named dangerous_raw_gms to flag intent.

key_protocol: NSSAUserData storage

  • group_key_holders: BTreeMap<String, GroupKeyHolder> field with #[serde(default)] for backward compatibility.
  • group_pda_accounts: BTreeMap<AccountId, Account> local cache for decrypted group PDA state.
  • get_group_key_holder / insert_group_key_holder accessors.

wallet: mask-3 transaction construction

  • PrivateGroupPda { group_label, program_id, seed } variant on PrivacyPreservingAccount.
  • group_pda_preparation derives keys from GroupKeyHolder, computes AccountId::for_private_pda, reads PDA state from local cache.

Test program: group_pda_spender

  • Handles deposit (claim + balance transfer) and spend (direct balance modification + noop chained call for mask-3 binding).
  • Unit tests in nssa/src/state.rs verify both paths at the circuit level.

Integration test

  • Demonstrates GMS sealed distribution, key agreement, and forward secrecy after ratchet.

🧪 How to Test

RISC0_DEV_MODE=1 cargo test --release -p key_protocol
RISC0_DEV_MODE=1 cargo test --release -p nssa -- group_pda

🔗 Dependencies

📋 PR Completion Checklist

  • Complete PR description
  • Implement the core functionality
  • Add/update tests
  • Add/update documentation and inline comments

@moudyellaz moudyellaz force-pushed the moudy/feat-group-key-holder branch 2 times, most recently from 12ef35b to 4938373 Compare April 17, 2026 15:02
Base automatically changed from moudy/feat-private-pdas to main April 22, 2026 22:34
@moudyellaz moudyellaz force-pushed the moudy/feat-group-key-holder branch from 9822584 to 1fdc2da Compare April 22, 2026 22:54
@moudyellaz moudyellaz marked this pull request as ready for review April 22, 2026 23:33
@moudyellaz moudyellaz force-pushed the moudy/feat-group-key-holder branch from 58cf9ef to 636fc9d Compare April 27, 2026 00:46
@moudyellaz moudyellaz force-pushed the moudy/feat-group-key-holder branch from f8fd4c8 to 6504d1f Compare April 27, 2026 01:24
Copy link
Copy Markdown
Collaborator

@Pravdyvy Pravdyvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for PR. Left a couple of questions.

Comment thread integration_tests/tests/group_pda.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs
Comment thread key_protocol/src/key_management/group_key_holder.rs
@moudyellaz moudyellaz force-pushed the moudy/feat-group-key-holder branch from 6504d1f to 9927e6e Compare April 27, 2026 12:43
Copy link
Copy Markdown
Collaborator

@Pravdyvy Pravdyvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Collaborator

@schouhy schouhy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Leaving some thoughts while I finish reviewing

Comment thread nssa/src/state.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread nssa/src/state.rs
Comment thread key_protocol/src/key_protocol_core/mod.rs Outdated
Copy link
Copy Markdown
Collaborator

@Arjentix Arjentix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Left some small comments.

I didn't quite get how it works, probably more examples like k/nsignatures program and an integration test demonstrating it would be nice

Comment thread key_protocol/src/key_protocol_core/mod.rs Outdated
Comment thread key_protocol/src/key_protocol_core/mod.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread test_program_methods/guest/src/bin/private_pda_spender.rs
Comment thread wallet/src/lib.rs Outdated
Comment thread nssa/src/state.rs Outdated
Comment thread nssa/src/state.rs Outdated
Comment thread nssa/src/state.rs Outdated
@moudyellaz
Copy link
Copy Markdown
Collaborator Author

moudyellaz commented Apr 29, 2026

group_pda_spender in test_program_methods/ is the example program. A full integration test that covers deposit → spend requires membership proof resolution for existing private PDAs, which the wallet doesn't support for group PDAs yet. The unit tests in circuit.rs verify both paths at the circuit level. A more complete integration test will follow once the wallet can resolve group PDA membership proofs. @Arjentix

@moudyellaz moudyellaz requested review from Arjentix and schouhy April 29, 2026 12:41
Copy link
Copy Markdown
Collaborator

@schouhy schouhy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey thanks. Have some general comments about the approach

Comment thread artifacts/test_program_methods/group_pda_router.bin Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread key_protocol/src/key_management/group_key_holder.rs Outdated
Comment thread key_protocol/src/key_protocol_core/mod.rs Outdated
Comment thread test_program_methods/guest/src/bin/private_pda_spender.rs
Comment thread wallet/src/lib.rs Outdated
- Add SealingPublicKey/SealingSecretKey type aliases for seal_for/unseal
- Generalize PrivateGroupPda to PrivatePda with pre-resolved keys
- Rename group_pda_spender to private_pda_spender
- Rename group_pda_accounts to pda_accounts with serde alias
- Remove unused storage_mut()
- Remove stale group_pda_router.bin artifact
@moudyellaz moudyellaz requested a review from schouhy April 30, 2026 07:25
Copy link
Copy Markdown
Collaborator

@schouhy schouhy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks LGTM. I'd personally remove the epoch, ratcheting and change the private_pda_spender to use the authenticated transfer. But I don't insist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants